Skip to content

Fix CMAKE_ARGS variable expansion#101

Merged
h-vetinari merged 3 commits into
conda-forge:mainfrom
olivier-roussel:fix_cmake_args_expansion
Jul 21, 2025
Merged

Fix CMAKE_ARGS variable expansion#101
h-vetinari merged 3 commits into
conda-forge:mainfrom
olivier-roussel:fix_cmake_args_expansion

Conversation

@olivier-roussel
Copy link
Copy Markdown
Contributor

@olivier-roussel olivier-roussel commented Jul 21, 2025 •
edited
Loading

#99 broke the CMAKE_ARGS value, as new variable set here causes to invalidate previous set here. Then, the CMAKE_ARGS variable does not contain anymore the CMAKE_INSTALL_PREFIX and so values.
This looks most probably due to the lack of delayed variable expansion in the bat file.

Thanks to @jorisv for the help on this investigation.

Checklist

@olivier-roussel olivier-roussel marked this pull request as draft July 21, 2025 12:49
@conda-forge-admin
Copy link
Copy Markdown
Contributor

conda-forge-admin commented Jul 21, 2025 •
edited
Loading

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe/meta.yaml) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe/meta.yaml:

  • ℹ️ The recipe is not parsable by parser conda-souschef (grayskull). This parser is not currently used by conda-forge, but may be in the future. We are collecting information to see which recipes are compatible with grayskull.
  • ℹ️ The recipe is not parsable by parser conda-recipe-manager. The recipe can only be automatically migrated to the new v1 format if it is parseable by conda-recipe-manager.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/16418166152. Examine the logs at this URL for more detail.

@h-vetinari
Copy link
Copy Markdown
Member

h-vetinari commented Jul 21, 2025

Thanks for the PR!

If you want the changes to take effect, you'll very likely need to use !CMAKE_ARGS! instead of %CMAKE_ARGS% in

vc-feedstock/recipe/activate.bat

Lines 90 to 95 in 7800961

set "CMAKE_ARGS=-DCMAKE_BUILD_TYPE=Release"
IF "%CONDA_BUILD%" == "1" (
set "CMAKE_ARGS=%CMAKE_ARGS% -DCMAKE_INSTALL_PREFIX=%LIBRARY_PREFIX% -DCMAKE_PROGRAM_PATH=%BUILD_PREFIX%\bin;%BUILD_PREFIX%\Scripts;%BUILD_PREFIX%\Library\bin;%PREFIX%\bin;%PREFIX%\Scripts;%PREFIX%\Library\bin"
:: see https://github.com/conda-forge/conda-smithy/issues/2319
set "CMAKE_ARGS=%CMAKE_ARGS% -DPython_FIND_REGISTRY=NEVER -DPython3_FIND_REGISTRY=NEVER"
)

as %VAR% is evaluated when the line is first parsed

@olivier-roussel
Copy link
Copy Markdown
Contributor Author

olivier-roussel commented Jul 21, 2025

Indeed, I clearly misunderstood the delayed expansion behavior, thanks for the clarification.
So, if I understood now correctly, just delaying the call on this line shoud be enough:

vc-feedstock/recipe/activate.bat

Line 94 in 7800961

set "CMAKE_ARGS=%CMAKE_ARGS% -DPython_FIND_REGISTRY=NEVER -DPython3_FIND_REGISTRY=NEVER"

@olivier-roussel olivier-roussel marked this pull request as ready for review July 21, 2025 13:26
@h-vetinari
Copy link
Copy Markdown
Member

h-vetinari commented Jul 21, 2025

So, if I understood now correctly, just delaying the call on this line shoud be enough:

I would have just put it on both lines, but this should be fine.

@h-vetinari h-vetinari merged commit 831f9fe into conda-forge:main Jul 21, 2025
13 checks passed
@olivier-roussel olivier-roussel mentioned this pull request Jul 21, 2025
Closed
1 task
@h-vetinari h-vetinari mentioned this pull request Jul 21, 2025
Merged
@pauldmccarthy pauldmccarthy mentioned this pull request Jul 22, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@183amir 183amir Awaiting requested review from 183amir 183amir is a code owner

@h-vetinari h-vetinari Awaiting requested review from h-vetinari h-vetinari is a code owner

@isuruf isuruf Awaiting requested review from isuruf isuruf is a code owner

@jakirkham jakirkham Awaiting requested review from jakirkham jakirkham is a code owner

@msarahan msarahan Awaiting requested review from msarahan msarahan is a code owner

@patricksnape patricksnape Awaiting requested review from patricksnape patricksnape is a code owner

@pelson pelson Awaiting requested review from pelson pelson is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@olivier-roussel @conda-forge-admin @h-vetinari